Introduce generic XY endpoint #132
Conversation
siwei-zhang-work
force-pushed
the
generic-xy
branch
2 times, most recently
from
f7735fd to
b6a1fbf
Compare
bhufmann
left a comment
bhufmann
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I reviewed and found a couples things to change.
API-proposed.yaml
Outdated
| format: int32 | ||
| filter_query_parameters: | ||
| $ref: "#/components/schemas/RequestedFilterQueryParameters" | ||
| RequestedFilterQueryParameters: |
There was a problem hiding this comment.
RequestedFilterQueryParameters already exists in this file (line 3423) and doesn't need to be added again.
API.yaml
Outdated
| format: int32 | ||
| filter_query_parameters: | ||
| $ref: "#/components/schemas/RequestedFilterQueryParameters" | ||
| RequestedFilterQueryParameters: |
There was a problem hiding this comment.
RequestedFilterQueryParameters already exists in this file (line 2902) and doesn't need to be added again.
API-proposed.yaml
Outdated
| $ref: "#/components/schemas/OutputElementStyle" | ||
| xValues: | ||
| xValuesDescription: | ||
| type: array |
There was a problem hiding this comment.
this is not correct, I think. It's not an array. The swagger documentation needs to be updated for that and the API regenerated.
API-proposed.yaml
Outdated
| axisDomain: | ||
| $ref: "#/components/schemas/AxisDomain" | ||
| yValuesDescription: | ||
| type: array |
There was a problem hiding this comment.
this is not correct, I think. It's not an array. The swagger documentation needs to be updated for that and the API regenerated.
API.yaml
Outdated
| xValuesDescription: | ||
| type: array | ||
| description: Series' X values | ||
| description: Series' X axis description |
There was a problem hiding this comment.
same comment as for API_proposed.yaml
| description: Label for the axis | ||
| axisDomain: | ||
| $ref: "#/components/schemas/AxisDomain" | ||
| yValuesDescription: |
There was a problem hiding this comment.
same comment as for API_proposed.yaml
This endpoint is introduced to support xy chart that has a non-time x-axis proposed in eclipse-cdt-cloud#114 Signed-off-by: Siwei Zhang <[email protected]>
siwei-zhang-work
force-pushed
the
generic-xy
branch
from
b6a1fbf to
8eab3f0
Compare
|
Fixed and created PR in incubator in: https://github.com/eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator/pull/215/files |
bhufmann
left a comment
bhufmann
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for the update.
API-proposed.yaml
Outdated
| properties: | ||
| type: | ||
| type: string | ||
| description: "Type of axis domain (e.g., 'categorical' or 'timeRange')" |
API-proposed.yaml
Outdated
| type: | ||
| type: string | ||
| description: Type of axis domain | ||
| AxisDomainTimeRange: |
API.yaml
Outdated
| properties: | ||
| type: | ||
| type: string | ||
| description: "Type of axis domain (e.g., 'categorical' or 'timeRange')" |
API.yaml
Outdated
| type: | ||
| type: string | ||
| description: Type of axis domain | ||
| AxisDomainTimeRange: |
API-proposed.yaml
Outdated
| type: string | ||
| description: Label for the axis | ||
| axisDomain: | ||
| $ref: '#/components/schemas/AxisDomain' |
There was a problem hiding this comment.
I think this should be oneOf { AxisDomainCategorical, AxisDomainRange }, either here or directly in AxisDomain definition?
There was a problem hiding this comment.
@siwei-zhang-work I think if you change the swagger annotation for AxisDomain to the following, then it will achieve what Patrick suggests:
@Schema(description = "Domain of values supported on a chart axis. Can be either categorical or numeric range.", oneOf = {
AxisDomainCategorical.class,
AxisDomainRange.class
})I uploaded the corresponding PR in the Trace Compass server code:
eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#220
|
@siwei-zhang-work I pushed a commit on top of your branch used to generate this PR to include the incubator update and the fixes that Patrick pointed out. |
Signed-off-by: Bernd Hufmann <[email protected]>
bhufmann
left a comment
bhufmann
left a comment
There was a problem hiding this comment.
@siwei-zhang-work changes look good. Thanks!
3 participants
This endpoint is introduced to support xy chart that has a non-time x-axis proposed in
#114
What it does
Introduce generic XY endpoint proposed in #114